feat(rspeedy): lower let/const to var in output for QuickJS parse speed#2755
Conversation
Add the SWC `transform-block-scoping` pass to both the background and main-thread layers (on top of the existing target baseline, outside `getESVersionEnvInclude` so the target-equivalence guard stays valid), and set rspack `output.environment.const = false` so bundler-generated runtime/wrapper code also emits `var`. Together these convert every `let`/`const` in the build output to `var`, which QuickJS parses faster. This intentionally changes emitted output (var instead of let/const).
🦋 Changeset detectedLatest commit: af0621d The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds SWC's transform-block-scoping to core and React plugin SWC configs and configures Rspack to emit var instead of const in bundler runtime; includes tests and a changeset documenting minor package bumps. ChangesConst-to-var lowering implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
@lynx-js/rspeedy
create-rspeedy
@lynx-js/react-rsbuild-plugin
@lynx-js/react-alias-rsbuild-plugin
upgrade-rspeedy
commit: |
Merging this PR will not alter performance
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | 004-various-update__main-thread-setAttribute__ListItemPlatformInfoSpread |
100.1 µs | 117.1 µs | -14.51% |
| ❌ | 003-hello-list/main-thread.js_LoadScript |
2 ms | 2.1 ms | -7.04% |
| ❌ | 004-various-update__main-thread-setAttribute__Spread |
387.4 µs | 414.2 µs | -6.47% |
| ❌ | 006-static-raw-text__main-thread-renderMainThread |
5 ms | 5.3 ms | -5.7% |
| ❌ | 003-hello-list__main-thread-renderMainThread |
19.6 ms | 20.7 ms | -5.47% |
| ❌ | 003-hello-list/background.js_LoadScript |
1.9 ms | 2 ms | -5.16% |
| ⚡ | 009-eval-bench/background.js_LoadScript |
888.8 µs | 626.4 µs | +41.89% |
| ⚡ | more faster fib(20) |
18.9 µs | 14.8 µs | +27.96% |
| ⚡ | transform 1000 view elements |
47.2 ms | 40.3 ms | +17.01% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/lower-const-let-to-var (af0621d) with main (b9d0624)2
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(a3e4607) during the generation of this report, so b9d0624 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
UI JudgeGEQI weighted score: 62 / 100 across 8 examples.
DetailsResult 1
Result 2
Result 3
Result 4
Result 5
Result 6
Result 7
Result 8
|
The prior CodSpeed run compared an AMD-EPYC base against an Intel-Xeon head; this empty commit forces a fresh run hoping for a same-CPU base so the simulation deltas are comparable.
…n CI (#2756) ## Summary The `Vitest (Ubuntu)` job intermittently fails on `plugin-qrcode`: ``` packages/rspeedy/plugin-qrcode/test/index.test.ts:198 (and :275) Error: Matcher did not succeed in time. expected "renderUnicodeCompact" to be called 2 times, but got 1 times ``` Both sites do `await expect.poll(() => renderUnicodeCompact).toBeCalledTimes(2)`, waiting for the QR re-render triggered when an interactive-prompt promise resolves (`resolve('bar')` / `resolve('main2')`). They used `expect.poll`'s **default 1000ms** timeout, which busy CI runners routinely exceed, so the poll gives up before the 2nd render lands. Locally (fast machine) it passes 9/9. The dev-server tests also ran on the default **5s** test budget, even though `waitDevCompileDone` alone allows up to 5s — so simply bumping the poll timeout would just hit the test timeout instead. ## Failing CI (before this fix) - `Vitest (Ubuntu)` on #2755: https://github.com/lynx-family/lynx-stack/actions/runs/26627685311/job/78469717405 — fails with "Matcher did not succeed in time" at `index.test.ts:198` and `:275`. ## Fix - `vitest.config.ts`: raise the package `testTimeout` to 20s (these tests legitimately spin up a real dev server). - The two re-render polls: pass `{ timeout: 10_000 }`. Test-only change (no version bump). Can't be reproduced locally (passes here), so the validation is CI staying green. ## Verified fixed - `Vitest (Ubuntu)` on this PR now passes: https://github.com/lynx-family/lynx-stack/actions/runs/26629013150/job/78474014792 (The `Vitest (Windows)` failure on this PR is an unrelated CI-infra issue — `hashFiles(...)` in the workflow template timing out on the Windows runner — not a test failure.) ## Test plan - [x] `Vitest (Ubuntu)` passes. - [ ] `Vitest (Windows)` — re-run to clear the unrelated `hashFiles` infra timeout.
Place the `output.environment.const: false` default first in the mergeRsbuildConfig chain so user-supplied `tools.rspack.output.environment.const` can override it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b35e87937d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b35e879 to
d4b61b4
Compare
Listing `'transform-block-scoping'` in `tools.swc.env.exclude` now skips the let/const → var lowering while leaving the rest of the baseline intact. Pairs with the runtime/wrapper opt-out from the rspack `output.environment.const` config.
d4b61b4 to
a3973f4
Compare
Spread `swcLoaderOptions.env` so the user's `env.exclude` reaches the main-thread SWC loader; SWC gives exclude precedence, so listing `'transform-block-scoping'` in `tools.swc.env.exclude` now opts both layers out of the let/const → var lowering.
Summary
Convert every
let/constin the build output tovar(QuickJS parsesvarfaster). Two halves:transform-block-scopingpass to both the background and main-thread layers, prepended on top of the existing target baseline. It is deliberately kept outsidegetESVersionEnvInclude(which mirrors exactly whatjsc.targetlowered, and is locked by a guard test), sincejsc.targetkeepslet/const.output.environment.const = false, so the runtime/wrapper code rspack emits also usesvar.This is an intentional output change (var instead of let/const), applied as a rspeedy-core default for all builds.
Verification
const a = 1; let b = 2→var a = 1; var b = 2.transform-block-scopingpresent in both layers'env.include;output.environment.const === false; snapshots updated. ThegetESVersionEnvInclude↔jsc.targetequivalence guard still passes (block-scoping is added separately).examples/reactbuilds clean.Test plan
turbo build(example builds) +turbo api-extractorchangeset statusshows@lynx-js/rspeedy+@lynx-js/react-rsbuild-pluginminorPerformance validation (empirical)
Measured
varvsconst/leton the real Lynx engine (PrimJS, viabenchx_cli) — same machine, var and const/let bundles run interleaved (N=30) to cancel timing drift; walltime read from the perfetto trace viatrace_processor:executeLoadedScript— chunk top-level execution (theLoadScriptregion)prepareAndEvalScript— prepare + evalPlus isolated parse+compile on QuickJS (PrimJS's parent engine;
std.evalScript(compile_only), 1.2 MB minified, identical SWC codegen, N=120, reproduced twice):varis ~3–5% faster (min).So in walltime — what actually affects on-device load time —
varis faster across parse, eval, and full chunk execution (~1–5%). Modest but consistent.On the CodSpeed check
CodSpeed flags
…_LoadScript −7%, but that is an instruction-count number, not walltime. Those*_LoadScriptbenchmarks come frombenchmark/react/plugins/pluginScriptLoad.mjs, which injectsCodspeed.startBenchmark()/stopBenchmark()around each chunk's top-level execution (gated ontypeof Codspeed !== "undefined", so they only exist under CodSpeed). CodSpeed counts instructions in that region; the same region measured locally in walltime (executeLoadedScript, table above) is faster forvar. In other wordsvarruns slightly more instructions but in less time — the instruction-count proxy diverges from real time here. The reported run was also cross-environment (base/head on different CPUs and memory, with an implausiblefib +28%swing), which further reduces its reliability. The flagged regressions can be acknowledged on CodSpeed.Summary by CodeRabbit
New Features
Tests